Skip to content

Automated Test: unified-storage-enhancements #338

Conversation

admin-coderabbit
Copy link
Owner

@admin-coderabbit admin-coderabbit commented Feb 4, 2026

This pull request was automatically created by @coderabbitai/e2e-reviewer.

Batch created pull request.

Summary by CodeRabbit

  • Improvements

    • Server initialization now occurs at startup instead of on first request, enabling earlier error detection.
    • Added initialization timing metrics to operational logs.
  • Tests

    • Improved test compatibility for postgres database environments.

…#97529)

* dont lazy init unified storage

* Inits index when creating new resource server. Fixes trace propagation by passing span ctx. Update some logging.

* Use finer grained cache locking when building indexes to speed things up. Locking the whole function was slowing things down.

* formatting

* linter fix

* go mod

* make update-workspace

* fix workspaces check error

* update dependency owner in mod file

* wait 1 second before querying metrics

* try with big timeout, see if fixes CI. Wont fail locally.

* skips postgres integration test. Only fails in drone. Will fix later.

* put delay back to 500 ms
@coderabbit-eval
Copy link

coderabbit-eval bot commented Feb 4, 2026

📝 Walkthrough

Walkthrough

This pull request transitions the resource storage system from lazy to eager initialization, refactors logger usage to use local loggers instead of external packages, updates context propagation in span handling, and adjusts cache synchronization timing in the search index. A test skip guard is also added for postgres environments.

Changes

Cohort / File(s) Summary
Logger and Tracing Updates
pkg/storage/unified/resource/search.go, pkg/storage/unified/sql/backend.go
Removed external logger dependency and replaced with local logger in search.go; updated tracer.Start calls to capture and propagate context (ctx, span) instead of discarding context; added initialization and latency logging metrics.
Search Index Management
pkg/storage/unified/search/bleve.go
Relocated cache locking to end of BuildIndex instead of beginning; replaced log message on successful disk index creation with TODO comment; adjusted synchronization timing for cache mutations.
Initialization Architecture
pkg/storage/unified/resource/server.go
Changed from lazy per-request initialization to eager upfront initialization in NewResourceServer; reordered init steps so watcher starts after search index is ready; removed Init calls from all RPC handlers (Create, Update, Delete, Read, List, Watch, Search, History, Origin, IsHealthy, PutBlob, GetBlob).
Test Coverage
pkg/server/module_server_test.go
Added runtime skip guard for postgres in TestIntegrationWillRunInstrumentationServerWhenTargetHasNoHttpServer alongside existing sqlite skip.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant ResourceServer
    participant SearchIndex
    participant Watcher
    participant RPCHandler

    Note over Client,RPCHandler: Old Behavior (Lazy Initialization)
    Client->>ResourceServer: Call RPC (e.g., Create)
    activate ResourceServer
    ResourceServer->>ResourceServer: Init() on first request
    ResourceServer->>SearchIndex: Initialize search
    ResourceServer->>Watcher: Initialize watcher
    ResourceServer->>RPCHandler: Continue with request
    deactivate ResourceServer

    Note over Client,RPCHandler: New Behavior (Eager Initialization)
    Client->>ResourceServer: NewResourceServer()
    activate ResourceServer
    ResourceServer->>SearchIndex: Initialize search immediately
    ResourceServer->>Watcher: Start watcher after search ready
    ResourceServer->>ResourceServer: Init completes with error handling
    deactivate ResourceServer
    
    Client->>RPCHandler: Call RPC (e.g., Create)
    activate RPCHandler
    RPCHandler->>RPCHandler: Execute request directly (no Init)
    deactivate RPCHandler
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Eager now, we initialize with haste,
No lazy waits, no wasted time or waste,
The search prepares, the watcher stands in line,
Context flows through spans so fine,
Ready to serve before the first request chime! 🌟

🚥 Pre-merge checks | ✅ 1 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Automated Test: unified-storage-enhancements' is vague and generic, using 'Automated Test' as a prefix that obscures the actual technical changes. Replace with a descriptive title summarizing the main changes, such as 'Eagerly initialize resource server and reorder initialization steps' or 'Refactor unified storage initialization and logging'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch unified-storage-enhancements

Comment @coderabbit eval help to get the list of available commands and usage tips.

@coderabbit-eval coderabbit-eval bot closed this Feb 4, 2026
Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant